feat(copilot): add GitHub Copilot provider (via @github/copilot-sdk) - #7656

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

_{Comment @coderabbitai help to get the list of available commands.}

Thanks for the automated review — addressed the findings (force-pushed c10b8ef). Typecheck + lint clean, 75 provider/text-gen tests green.

Correctness

• CopilotAdapter consumer fiber (High) — forked with Effect.forkChild, so it was tied to the startSession fiber and interrupted on return. Now Effect.forkIn(adapterScope); torn down explicitly in stopSessionInternal.
• Concurrent sendTurn (High) — a second turn overwrote activeTurnCompletion and misattributed events. Now rejects a second in-flight turn (A turn is already in progress) rather than serialize across the whole turn, since stopSession/interruptTurn must run mid-turn.
• ServerSettingsPatch missing copilot (High) — added CopilotSettingsPatch and registered it in the patch provider map, so binaryPath/enabled persist.
• Stale activeTurnId (Medium) — cleared both ctx.activeTurnId and the session's activeTurnId on completion so listSessions() doesn't report an idle session as active.
• refreshInterval override (Medium) — removed; Copilot now follows providerHealthRefreshInterval like the other providers.
• parseCopilotVersionOutput missing-binary (Medium) — the nonzero-exit branch now also matches not found/enoent, so the install guidance is surfaced (added a test).

Effect conventions

• CopilotTextGeneration now uses Schema.is(TextGenerationError) and Effect.catchTags, and drops the pass-through error factory (matches CursorTextGeneration).
• CopilotSdkError aligned to { operation, detail, cause } like OpenCodeRuntimeError; adapter errors use a static detail + cause instead of folding cause.message.

One I left as-is: the Failed to build Copilot snapshot: ${cause.message} detail in CopilotDriver — that mirrors GrokDriver's exact pattern, so I kept it consistent with the sibling. Happy to change if you'd prefer otherwise.

Second pass on the re-review (45ca7b9):

• Native event logging — the nativeEventLogger was accepted but never fed. Added a logNative helper (mirroring GrokAdapter) and now record each SDK session event and permission request/completion, so nativeEventLogPath / the injected logger capture Copilot traffic.
• resolveCopilotSdkTunables validation — now validates reasoning effort against the allowed set (none/low/medium/high/xhigh/max) and drops boolean/invalid values, matching the context-tier path and the documented guarantee (added tests).
• Adapter error details — the getClient / createSession mappings now use stable structural phrases ("Failed to start the Copilot SDK runtime client." / "Failed to create or resume the Copilot SDK session.") with the raw cause preserved, instead of copying the SDK failure string.

Typecheck + lint clean, 76 tests green.

Round 3 (1e7bc5722):

• interruptTurn could strand a turn (High) — if abort() rejected, no session.idle arrived and sendTurn blocked forever. Now completes the active turn as aborted after abort() settles (idempotent, so a normal abort that emits session.idle just no-ops).
• send rejection left a stale active turn (Medium) — the error path now clears activeTurnId and session.activeTurnId too, not just activeTurnCompletion.
• Auth env precedence (Medium) — a blank COPILOT_GITHUB_TOKEN no longer masks a real GH_TOKEN/GITHUB_TOKEN; picks the first non-blank token in order (with a test).

Not changed — flagging as consistent-with-siblings rather than a Copilot-specific bug:

• Update executable ignores binaryPath (CopilotDriver:53) — makeStaticProviderMaintenanceResolver hard-codes the executable, but GrokDriver and ClaudeDriver do exactly the same (grok/claude). Deriving the update binary from a custom binaryPath looks like a repo-wide maintenance-resolver change rather than something to fix only for Copilot — happy to do it separately if you'd like it repo-wide.

Typecheck + lint clean, 77 tests green.

Round 4 (a91e1c58b): turn cleanup is now interruption-safe — moved the active-turn reset (activeTurnCompletion / activeTurnId / session.activeTurnId) into an Effect.ensuring finalizer around the send + Deferred.await. Previously an interrupted or failed sendTurn left activeTurnCompletion set, wedging the session (all later turns rejected as already-in-progress). Now it clears on success, failure, or interruption alike. Typecheck + lint clean, 533 provider tests green.

Round 5 (6d40d6ef5): CopilotSdkError now follows the provider/Errors.ts convention — Schema.TaggedErrorClass with cause: Schema.Defect() and a message derived from structural attributes (operation + detail), matching ProviderAdapterRequestError et al. (was Data.TaggedError). Typecheck + lint clean, 77 tests green.

I believe that leaves only the CopilotDriver:53 update-executable note, which I've kept consistent with GrokDriver/ClaudeDriver (all static-executable resolvers) rather than diverging one provider — flagged above as a repo-wide follow-up if you want it.

Round 6 (6cd5f28b6): tool calls now emit the canonical item.started on tool.execution_start (was item.updated), so consumers see the start of the lifecycle; progress → item.updated, completion → item.completed.

Round 7 (525c76bda):

• CopilotSdkError — dropped the detail field; message is now derived purely from the structural operation attribute, with the real failure preserved on cause (matches the provider/Errors.ts models).
• Formatting — ran vp fmt across all touched files; the earlier ~80-col re-wraps in providerDriverMeta.ts, model.ts, settings.ts, builtInDrivers.ts, etc. are back to the 100-col default, so fmt:check is clean.

All three Macroscope review checks are green now (Correctness, Effect Service Conventions, UI Consistency). The only remaining red check is Vercel – t3code-marketing ("Authorization required to deploy") — that's the org deploy gate for an external-fork PR, not something in this diff. Ready for a human look whenever you have a moment; happy to keep iterating on anything else.

^ Leave it to Claude to make my comments sound AI-generated as hell

Round 11 (3a935573e):

• Version check no longer trusts arbitrary output (High) — a zero-exit copilot version whose output has no parseable version now reports warning with a diagnostic instead of ready, so a mis-pointed binaryPath isn't treated as a healthy install (test added).
• Release force-stop (High) — scope closure now falls back to forceStop() if stop() rejects, matching the acquisition path, so a failed graceful shutdown can't leave the runtime child process alive.

Typecheck + lint clean.

Round 12 (0b546b10a):

• Stale turn.completed after stop/replace (Medium) — when a session is stopped or replaced mid-turn, sendTurn now returns without mutating the detached context or publishing a turn.completed (which would otherwise land after session.exited / a replacement's start events). Guarded on ctx.stopped, which stopSessionInternal already sets.
• Health-check message (style) — dropped the interpolated defect text; the non-missing-binary branch is now the stable phrase the sibling providers use (the failure is preserved on the error itself).

Typecheck + lint clean, 534 provider tests green.

Round 13 (21f5b95d6): health probe now resolves the binary like the SDK does — runCopilotVersionCommand was passing binaryPath straight to ChildProcess.make, so a GUI-launched process with a restricted PATH reported an installed CLI as missing (even though sessions worked, since the SDK resolves it). Exposed the resolver as resolveCopilotBinaryPath and use it for the version check too. Typecheck + lint clean.

Round 14 (0c26e20a9):

• Binary resolver rejects directories (High) — isExecutable now stats the candidate and requires a regular file, so a directory named copilot earlier in PATH can't shadow a real CLI later in PATH.
• Tool item-type labeling (Medium) — grep/find/glob (local code searches) are no longer mislabeled as web_search; only actual web fetch/search tools map there, everything else falls through to dynamic_tool_call.

Typecheck + lint clean.

Round 15 (92461c7f7): rollbackThread now rewinds the actual SDK conversation, not just the local turn list. It uses session.rpc.history.listRewindPoints() + rewind({ eventId, mode: "conversation" }) (rewind points sorted by timestamp for order-safety), and only mirrors the local ctx.turns once the backend rewind returns outcome: "success" — otherwise it fails rather than falsely reporting a rollback that didn't happen. Typecheck + lint clean, 534 provider tests green.

(Also in this push: binary resolver now rejects directories via stat().isFile(), and grep/find/glob tool calls are no longer mislabeled web_search.)

Round 16 (aa37d5d35):

• Tool item-type preserved across the lifecycle (Medium) — the classification is computed once at tool.execution_start and remembered per toolCallId, so progress/completion no longer collapse command_execution/file_change/mcp_tool_call into a generic dynamic_tool_call.
• Text-gen no longer drops caller context (Medium) — commit-message/PR/thread-title generation now forward policy, changeRequestTemplate, and previousTitle (matching CursorTextGeneration).
• Respect enableProviderUpdateChecks (Medium) — the driver adopted the makeProviderSnapshotSettingsSource pattern and forwards the setting to enrichProviderSnapshotWithVersionAdvisory, so version advisories/npm checks are skipped when the user disables update checks.
• Snapshot error detail (style) — static structural phrase + cause (matches AetherDriver/OpenCode2Driver).

Typecheck + lint clean, 596 provider/text-gen tests green.

Two I'm leaving as consistent-with-siblings (not Copilot-specific), flagging rather than diverging one adapter:

• threadLocksRef growth (CopilotAdapter:238) — GrokAdapter and CursorAdapter use the identical per-thread semaphore map with no eviction; naive cleanup risks the lock races the map exists to prevent, so this is better solved repo-wide (e.g. ref-counted) than in one adapter.
• Update executable ignores binaryPath (CopilotDriver:53) — makeStaticProviderMaintenanceResolver hard-codes the executable in GrokDriver/ClaudeDriver too; happy to make it binaryPath-aware repo-wide if you'd like.

Round 17 (636565e03) — the two findings raised after round 16, plus @NaveDanan's remaining parity notes:

• Repeated model discovery on empty options (Medium, CopilotProvider.ts:409) — a discovered model with no tunable reasoning/context options left modelsNeedDiscovery stuck at true, so every enrichment cycle re-spawned an SDK client and re-ran listModels(). Discovery is the only source of non-custom models (fallback/initial snapshots carry none), so their presence alone now means discovery ran; an empty optionDescriptors is treated as a valid result rather than a retry signal.
• Windows binary resolution (High, CopilotSdkClient.ts) — resolveCopilotBinaryPath only probed the exact name, so a normal npm-installed copilot.cmd never resolved and the SDK got a bare copilot. It now probes PATHEXT suffixes (.CMD, …) on Windows (NodePath.sep as the lint-safe platform check) alongside the exact name. Added CopilotSdkClient.test.ts covering PATH resolution, the directory-shadow rejection, and bare-name fallback.
• Text-gen model options (parity) — the git text-generation path now forwards the caller's selected reasoningEffort / contextTier (resolveCopilotSdkTunables(modelSelection.options)) into the one-shot SessionConfig, matching the interactive adapter and CursorTextGeneration. (policy / changeRequestTemplate / previousTitle were already forwarded in round 16.)

Typecheck + lint + fmt clean; 20 Copilot provider/SDK tests green.

On the one item I've left as-is: threadLocksRef (CopilotAdapter.ts:238) retaining a semaphore per historical thread is byte-identical to GrokAdapter and CursorAdapter (verified). Per @NaveDanan's "I would not block this PR on broader behavior that is already shared by other providers," I've kept it consistent with the siblings rather than diverging one adapter — a safe eviction needs the same treatment across all three (naive cleanup risks a lock race where a concurrent op recreates the semaphore and loses mutual exclusion). Happy to do it as a repo-wide follow-up if you'd prefer.

@NaveDanan — thanks again for the thorough parity pass; that list was exactly right. The one thing I haven't added is a dedicated adapter test harness for concurrent turns / interruption / startup failure: the sibling adapter tests drive a mock ACP subprocess, which doesn't apply to the SDK transport, so that needs a mock copilot-sdk runtime — flagging it as a follow-up rather than blocking this on it.

Round 18 (e38346a9d) — the Macroscope finding on model discovery, plus several correctness fixes surfaced by an adversarial self-review of the diff before pushing.

The reported finding (CopilotProvider.ts model discovery)
Discovery had been running in background enrichment, so every status refresh reset models to the fallback [] and re-discovered later — dropping the catalog transiently, and permanently if discovery then failed. Moved discovery into checkCopilotProviderStatus (matching Cursor/Grok, which discover during the status check), and added a driver-scoped discoveredModelsRef that preserves the last-known-good catalog across a discovery timeout/empty/failure. Enrichment is now version-advisory only. This also retires round-17's "needs discovery" heuristic — the whole enrichment-discovery bug class is gone rather than patched.

Correctness fixes found while reviewing the above

• Provider errors now surface (was log-only). A root session.error was previously only logged — it neither emitted a runtime.error nor failed the turn, so a failed turn either reported completed or hung on its Deferred.await. It now emits runtime.error{provider_error} + settles the turn failed (mirroring OpenCodeAdapter). Added a session.shutdown handler too (crash → transport_error + failed; routine → aborted), so a terminal event with no trailing session.idle can't wedge the thread. Sub-agent-scoped events (agentId set) never touch the root turn.
  • Note on rate_limit/eligibleForAutoSwitch: since T3 registers no auto-switch handler (and never sets continueOnAutoMode), the SDK auto-declines the switch and the turn produces no output — so these are treated as terminal (surfaced + failed) rather than silently completed. There's a comment marking the exact spot to revisit if an auto-switch handler is ever added.
• Uninterruptible client.start() bounded. start() runs in acquireRelease's uninterruptible acquire, so an Effect-level timeout can't cut a stalled handshake — and with discovery now on the refresh path, a hung start would freeze the provider's single refresh permit. Bounded it with a JS-level Promise.race. The failure/release cleanup is also bounded: the SDK's stop() resolves-with-errors rather than rejecting on a hang, so the old .catch(() => forceStop()) never fired — now it races stop() against a 2s timer and hard-kills on timeout. Discovery uses an 8s start cap so a wedged start + cleanup stays within the ~10s discovery budget (interactive/text-gen keep the generous 15s default).
• No runtime spawn for a broken CLI. Discovery now gates on parsed.status === "ready".

Deliberately not done: a blanket per-turn timeout on Deferred.await (it would kill legitimately long agentic turns; there's no SDK disconnect/process-death callback to hook instead — verified — so terminal-event handling is the mechanism). Flagging it in case you'd prefer a different backstop.

Typecheck (0 errors, no new warnings) + lint + fmt clean; 20 provider/SDK tests green.

Round 19 (e0795ed7d) — the three findings from the last automated pass.

• Rejected turns no longer mutate the session (Medium). sendTurn now validates attachments and the prompt before calling applyModelSelection, so a turn rejected for an invalid attachment or empty prompt no longer leaves a new model/reasoning-effort applied to the live session.
• Empty PATH component honored (High). resolveCopilotBinaryPath was dropping empty PATH components via .filter(Boolean), so a copilot reachable through a POSIX empty component (which means the current directory) went unfound and the bare name was handed to the SDK (which does no PATH lookup). Empty components now resolve to the CWD, evaluated lazily so a normal PATH never touches it.
• Interrupt→new-turn race (High). The SDK's session.idle/abort events are session-scoped and carry no turn id, so a delayed terminal event from an interrupted turn could settle a newer turn's deferred. Fixed by draining rather than correlating:
  • session.idle is now the sole terminal settle point; the bare abort event no longer settles (an idle with aborted: true follows it).
  • interruptTurn no longer force-settles on a clean abort() ack — it lets the turn's own aborted session.idle settle it. Because the one-turn guard stays closed until that idle settles the turn, no later turn can be admitted while the terminal event is still in flight, so a session-scoped idle can never mis-settle a newer turn (and nothing needs to be swallowed). It only force-settles when abort() is not acknowledged (no idle guaranteed → avoids stranding the turn). A side benefit: a turn stays honestly "running" while an attached shell command the abort didn't kill finishes, instead of a premature completed.

The residuals are narrow and, I think, acceptable: an unacknowledged abort() only rejects on a dead connection (which drives session.shutdown/session.error instead of a stray idle), and a truly hung runtime that emits no terminal event leaves the turn honestly in-progress until the session is stopped (not a lying completed). Happy to revisit if you'd prefer a hard interrupt timeout.

Typecheck (0 errors, no new warnings) + lint + fmt clean; 20 provider/SDK tests green.

I opened a stacked follow-up PR: NSExceptional/t3code#1.

While testing the provider, I found two integration gaps:

• Copilot skill and config discovery was enabled for the SDK session, but T3 did not publish the discovered skills to its provider snapshot, so /skills searched the workspace instead.
• T3 listed Copilot slash commands but sent them through session.send; Copilot requires session.rpc.commands.invoke, so /skills was interpreted by the model as a normal prompt.

The follow-up adds skill metadata discovery, built-in command discovery, command invocation, command-result rendering, and focused regression tests. I tested /skills end-to-end through T3 after the change.

Closing this PR after an automated pass over open pull requests. Outside Copilot integration that competes with three other outside proposals and product-owned provider work.

@t3dotgg can you elaborate on what that means? Is this something only you or the core team will be able to implement? Or is it something you have in progress already?